Allow opening a database using a file handle instead of a path #665
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Having read the implementation of diff --git a/src/db.rs b/src/db.rs
index 586bd98..57f10cb 100644
--- a/src/db.rs
+++ b/src/db.rs
@@ -12,7 +12,6 @@ use crate::{
use crate::{ReadTransaction, Result, WriteTransaction};
use std::fmt::{Display, Formatter};
use std::fs::{File, OpenOptions};
-use std::io;
use std::io::ErrorKind;
use std::marker::PhantomData;
use std::ops::RangeFull;
@@ -769,20 +768,19 @@ impl Builder {
/// Opens an existing redb database.
pub fn open(&self, path: impl AsRef<Path>) -> Result<Database, DatabaseError> {
- if !path.as_ref().exists() {
- Err(StorageError::Io(ErrorKind::NotFound.into()).into())
- } else if File::open(path.as_ref())?.metadata()?.len() > 0 {
- let file = OpenOptions::new().read(true).write(true).open(path)?;
- Database::new(
- file,
- self.page_size,
- None,
- self.read_cache_size_bytes,
- self.write_cache_size_bytes,
- )
- } else {
- Err(StorageError::Io(io::Error::from(ErrorKind::InvalidData)).into())
+ let file = OpenOptions::new().read(true).write(true).open(path)?;
+
+ if file.metadata()?.len() == 0 {
+ return Err(StorageError::Io(ErrorKind::InvalidData.into()).into());
}
+
+ Database::new(
+ file,
+ self.page_size,
+ None,
+ self.read_cache_size_bytes,
+ self.write_cache_size_bytes,
+ )
}
/// Open an existing or create a new database in the given `file`.could be used to avoid the extra system calls to |
|
ya, that diff seems like it would work. As long as there's a test for a missing and an empty file it should be safe to change it |
cberner
requested changes
Sep 3, 2023
|
Couple minor comments. Otherwise this seems good to me! |
This enables using this library with e.g. `cap-std` which avoids path-based access in favour of a capability-based approach.
I added test cases for these two error paths as a separate commit before the refactoring. |
cberner
approved these changes
Sep 3, 2023
|
Merged. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables using this library with e.g.
cap-stdwhich avoids path-based access in favour of a capability-based approach.I am not sure about the name, but
openwhile probably most fitting is already taken for the path-based approach. Alternatives I considered werefrom_file,access,mountandopen_or_init.I added another commit to ensure that the build is Clippy-clean before submitting the PR. Please let me if know if you like me to drop that from the PR as it is admittedly unrelated. (Bumping
fastrandresolves a few warnings about unnecessary mutability in the LMDB benchmark.)